Skip to content

Conversation

@Murat65536
Copy link
Contributor

Closes #8191

@Murat65536 Murat65536 requested a review from a team as a code owner September 16, 2025 20:24
@github-actions github-actions bot added the component: wpilibj WPILib Java label Sep 16, 2025
@calcmogul
Copy link
Member

calcmogul commented Sep 17, 2025

fyi, these changes can be ported to C++ as well, despite the issue specifically mentioning Java.

@Murat65536
Copy link
Contributor Author

Murat65536 commented Sep 20, 2025

fyi, these changes can be ported to C++ as well, despite the issue specifically mentioning Java.

@calcmogul
I have very little experience with C++ compared to Java and VS Code shows a bunch of errors in any C++ file I try to open. Might be because I'm on Linux.

I see this:

void TimedRobot::AddPeriodic(std::function<void()> callback,
units::second_t period, units::second_t offset) {

What I don't see is where second_t is defined. I also don't see what units is. Something external? I installed Windows. I see it now. Annoying that eveything's broken on Linux but I guess it makes sense. C++ isn't cross-platform until you make it behave that way.

@ThadHouse
Copy link
Member

What I don't see is where second_t is defined. I also don't see what units is. Something external? I installed Windows. I see it now. Annoying that eveything's broken on Linux but I guess it makes sense. C++ isn't cross-platform until you make it behave that way.

Linux definitely does work. However by default in allwpilib, the project intellisense generation doesn't run. You can run ./gradlew generateVsCodeConfig to force that generation.

@Murat65536
Copy link
Contributor Author

That makes more sense. There's probably a reason that it doesn't when I open up the project that I don't know. Maybe bad stuff happens if I close the project before the command completes?

@Murat65536
Copy link
Contributor Author

@calcmogul
Should I be using templates for this? Sorry if I'm missing something really easy or obvious. I don't have much experience with C++, especially in large projects.

@KangarooKoala
Copy link
Contributor

You can just use the SI units (units::second_t, units::hertz_t, units::meter_t, etc.) for the C++ version, since our C++ units library will automatically convert between compatible unit types. (For example, you could pass 1_ft as an argument to a method expecting units::meter_t)

Copy link
Contributor

@KangarooKoala KangarooKoala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

Copy link
Contributor

@KangarooKoala KangarooKoala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems okay to me for what it's worth, though you should probably run wpiformat -f wpilibc/src/main/native/cpp/TimedRobot.cpp to make sure the long constructor line doesn't need to be broken up. (If you haven't installed wpiformat, just run python3 -m pip install wpiformat)

@calcmogul
Copy link
Member

This PR is blocked on #8267.

@calcmogul calcmogul added the state: blocked Something is blocking action. label Oct 2, 2025
calcmogul
calcmogul previously approved these changes Oct 2, 2025
@calcmogul calcmogul removed the state: blocked Something is blocking action. label Oct 3, 2025
Copy link
Contributor

@KangarooKoala KangarooKoala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me, for what it's worth.

@Murat65536 Murat65536 requested a review from calcmogul October 13, 2025 21:48
@PeterJohnson PeterJohnson merged commit 8b99ad8 into wpilibsuite:main Oct 29, 2025
42 of 44 checks passed
@Murat65536 Murat65536 deleted the unit-overloads branch October 29, 2025 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[java] Use units for more input parameters

7 participants